[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign fabriziopandini for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

/test pull-cluster-api-provider-aws-e2e-blocking

Is this the recommended way of dealing with the reconciliation of the ProviderCluster when the owning Cluster is being deleted?

What are other providers doing in this occasion?

I had a quick look at CAPV and CAPG but they don't seem to be doing that:

• https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/blob/main/controllers/vspherecluster_reconciler.go
• https://github.com/kubernetes-sigs/cluster-api-provider-gcp/blob/main/controllers/gcpcluster_controller.go

^ cc. @chrischdi

I agree with @damdo on this.

From core CAPI perspective this is not defined clearly I think (did not find something clearly about this in the InfraCluster contract xref). But it also does not mention the deletion should be the way of the issue description. So I'd conclude it should not be like that.

From my point of view: the infracluster provider should take care of keeping the environment usable, so deletion can succeed successfully.

So for the sake of making deletion reliable, IMHO CAPA should continue to reconcile the AWSCluster. Otherwise this could lead to even take longer for getting deleted and maybe produce more costs.
E.g. if CAPI is not able anymore to drain machines in a normal way, or CSI is not able to detach volumes and CAPI's waiting for successful detachment, etc.

Do we have numbers on how much it could save in terms of CAPA resources and AWS API calls?
Maybe the reconciliation could be optimized in other ways instead to be more efficient, and not only for AWSCluster's that have their Cluster marked for deletion.

Is this the recommended way of dealing with the reconciliation of the ProviderCluster when the owning Cluster is being deleted?

What are other providers doing in this occasion?

I had a quick look at CAPV and CAPG but they don't seem to be doing that:

• https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/blob/main/controllers/vspherecluster_reconciler.go
• https://github.com/kubernetes-sigs/cluster-api-provider-gcp/blob/main/controllers/gcpcluster_controller.go

I don't think it's defined anywhere, it seems more like an oversight. But I don't see any downside of doing it this way, only the advantage of being more efficient.

I agree with @damdo on this.

From core CAPI perspective this is not defined clearly I think (did not find something clearly about this in the InfraCluster contract xref). But it also does not mention the deletion should be the way of the issue description. So I'd conclude it should not be like that.

From my point of view: the infracluster provider should take care of keeping the environment usable, so deletion can succeed successfully.

So for the sake of making deletion reliable, IMHO CAPA should continue to reconcile the AWSCluster. Otherwise this could lead to even take longer for getting deleted and maybe produce more costs. E.g. if CAPI is not able anymore to drain machines in a normal way, or CSI is not able to detach volumes and CAPI's waiting for successful detachment, etc.

Do we have numbers on how much it could save in terms of CAPA resources and AWS API calls? Maybe the reconciliation could be optimized in other ways instead to be more efficient, and not only for AWSCluster's that have their Cluster marked for deletion.

I'll close this if everyone is against it. Please, would you take care of adding this to the contract so that developers know what's mandatory from infra providers?